Skip to content

Conversation

@brianrourkeboll
Copy link
Contributor

@brianrourkeboll brianrourkeboll commented Aug 7, 2024

Description

Followup to #14827.

  • Avoid extra 'T option allocations in Array.Parallel.tryFindIndex, Array.Parallel.tryFind, and Array.Parallel.tryPick.

Checklist

  • Test cases added.
  • Performance benchmarks added in case of performance changes.
  • Release notes entry updated.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2024

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@brianrourkeboll
Copy link
Contributor Author

Hmm, a3a50c0 reduces allocations significantly, but while it is faster in many scenarios, it is slower in others, depending on the size of the array and the predicate. I may take that change out, unless we know which scenarios we'd prefer to optimize for.

@brianrourkeboll
Copy link
Contributor Author

Hmm, a3a50c0 reduces allocations significantly, but while it is faster in many scenarios, it is slower in others, depending on the size of the array and the predicate. I may take that change out, unless we know which scenarios we'd prefer to optimize for.

Removed that for now.

@brianrourkeboll brianrourkeboll marked this pull request as ready for review August 8, 2024 12:59
@brianrourkeboll brianrourkeboll requested a review from a team as a code owner August 8, 2024 12:59
@psfinaki
Copy link
Contributor

psfinaki commented Aug 8, 2024

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff, thank you!

Copy link
Contributor

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, however it needs the release notes updated.

@vzarytovskii vzarytovskii added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Aug 9, 2024
@vzarytovskii vzarytovskii merged commit 97a2a75 into dotnet:main Aug 9, 2024
@brianrourkeboll brianrourkeboll deleted the array-parallel-tweaks branch August 10, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants